HDFS-15201 SnapshotCounter hits MaxSnapshotID limit#1870
HDFS-15201 SnapshotCounter hits MaxSnapshotID limit#1870lokeshj1703 merged 5 commits intoapache:trunkfrom
Conversation
...doop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
Submitted new PR with the changes requested. |
|
💔 -1 overall
This message was automatically generated. |
...-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshotManager.java
Show resolved
Hide resolved
|
@karthikhw can you double check the failed tests? especially the snapshot tests. |
|
I've gone through all the usage of the snapshot id. The only concern i had was bitwise operations on the snapshot id, but i didn't find any. Widening the allowed range shouldn't be a problem. |
...doop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
...doop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
It looks test case failure is un-related to this issue. |
|
@arp7 @jojochuang Can you please review this change if you get sometime? |
|
The changes look good. Just a question: why changing getMaxSnapshotID() from -1 to -2? |
|
Changed getMaxSnapshotID() from -1 to -2 because of CURRENT_STATE_ID(Integer.MAX_VALUE - 1) that have -1 already. If SNAPSHOT_ID_BIT_WIDTH is 28 then we are ok with -1 but later change in SNAPSHOT_ID_BIT_WIDTH to 31 then user have to update getMaxSnapshotID() from -1 to -2. |
|
Let's keep it "-1" since we are using 28 for the moment. If there still a problem later on, we should think about what to do. We do not necessarily change it to 31 at that time. |
|
Thank you @szetszwo I changed back to -1. |
|
+1 the latest change looks good. Thanks @karthikhw |
lokeshj1703
left a comment
There was a problem hiding this comment.
@karthikhw Thanks for working on this! The changes look good to me. +1.
|
@karthikhw Thanks for the contribution! @jojochuang @mukul1987 @arp7 @szetszwo Thanks for reviewing the PR! I have committed it to master branch. |
At the beginning, SNAPSHOT_ID_BIT_WIDTH was setted 31, but later was setted 28. Why change SNAPSHOT_ID_BIT_WIDTH from 31 to 28? I think maybe:
|
(cherry picked from commit 5250cd6) Change-Id: Ibf48916c28f35e866d8b441af65de1a0b92b1733 (cherry picked from commit 20ea94d4a940cef35f0ff873dfaea19c6e5a7b83)
Jira: https://issues.apache.org/jira/browse/HDFS-15201
Users reported that they are unable to take HDFS snapshots and their snapshotCounter hits MaxSnapshotID limit. MaxSnapshotID limit is 16777215.
I think, SNAPSHOT_ID_BIT_WIDTH is too low. May be good idea to increase SNAPSHOT_ID_BIT_WIDTH to 31? to aline with our CURRENT_STATE_ID limit (Integer.MAX_VALUE - 1).